capture+materialize: Support Nonsensitive Field Overlays#3119
Conversation
willdonnelly
force-pushed
the
wgd/2026-06-04-overlay-aware-configs
branch
from
6f50475 to
3fd8931
Compare
|
I believe this is ready for review now. I've tested locally that with these changes a source-hello-world->materialize-postgres pipeline can be set up as normal when not using an overlay, and that an overlay can be added without re-encrypting a task config and (with a connector modified to mark those fields as nonsensitive) will be applied to the effective task config. I have not specifically tested this with both the v1 and v2 runtimes because I don't know how to toggle between those, but the actual code changes are basically identical between the two so I think that's probably sufficient testing just to make sure I didn't break something dumb but subtle. |
|
Hmm, the "Dekaf Test" CI failure appears to be because the |
This is necessary so that builds of this connector pick up the "tolerate unknown JSON fields" change from Flow commit 184db0e, which in turn is necessary so that Flow CI tests of estuary/flow#3119 can pass since the Dekaf test uses `source-http-ingest:dev` and currently it errors out because that PR adds a new field which it doesn't recognize.
willdonnelly
force-pushed
the
wgd/2026-06-04-overlay-aware-configs
branch
from
3fd8931 to
9dc610a
Compare
williamhbaker
left a comment
williamhbaker
left a comment
There was a problem hiding this comment.
LGTM, no blockers, a couple of things to consider as comments, and also concurring deployment:
As is, this PR will start to populate the new sealed config field right away. We've been bitten numerous times by new field additions where consumers use the strict pbjson structs that disallow unknown fields. As you noted with source-http-ingest, it hadn't yet been updated with the recent move to more lenient pbjson structs that do allow unknown fields. Anything else that hasn't yet been updated will similarly crash if it sees these populated fields.
Off the top of my head, that would be the other rust connectors: source/materialize-kafka, and all the ATF connectors (at least, I don't remember seeing us having updated those recently, but I might have missed it).
Since the new field only shows up on Open messages it is just limited to consumers of Open messages, which is only connectors.
I'm betting this would also break flowctl preview and flowctl raw materialize-fixture unless an up-to-date flowctl is used, built from at least this commit or later. This is a practical concern for connectors repo integration tests, unless they are always pulling/building the latest flowctl.
| Ok(()) | ||
| } | ||
|
|
||
| fn display_ptr(ptr: &json::Pointer) -> String { |
There was a problem hiding this comment.
Any reason not use the Display already implemented by json::Pointer?
There was a problem hiding this comment.
The only functional difference I can see is that using Display an overlay which attempts to modify the root object would produce an error message like overlay modifies location , which is not marked nonsensitive (since the RFC 6901 representation of the root is the empty string) whereas this helper function special-cased the root as / for readability in the error message.
That probably isn't sufficient reason to reimplement a hackier version of JSON Pointer stringification so I've removed this helper.
There was a problem hiding this comment.
Actually that's a dumb objection anyway, I've added single-quotes so that error would be overlay modifies location '', which is not marked nonsensitive which makes it pretty unambiguous to me.
| fn empty_overlay_is_a_noop() { | ||
| check(json!({})).unwrap(); | ||
| // An empty object writes nothing even at an otherwise-sensitive location. | ||
| check(json!({"credentials": {}})).unwrap(); |
There was a problem hiding this comment.
I can't think of a reason why this would need fixing, but if I'm understanding this correctly an empty object would kind of write something to a sensitive location if it were not of type object (like a string), replacing it instead with an object. In any sane world the resulting config would fail to parse in the connector, or otherwise not have any kind of exposure risk.
There was a problem hiding this comment.
The worst I can think of is a connector that had something like tls: true as a sensitive field and then somehow it thought tls: {} meant false 🤷
There was a problem hiding this comment.
Good catch, yeah, an empty object (or even a nested chain of objects as long as the final one is empty) passes the current nonsensitive-property validation but it probably shouldn't since merge(scalar, object) = merge({}, object).
I think to close that gap the validation rule needs to be something like "we only recurse into an object if the target schema permits an object at this location" and an object in a scalar location needs to be rejected.
|
Am I right in my understanding that this allows connectors to emit an update to a non-encrypted field (e.g. |
Thanks, I believe those should be addressed now.
Is your recommendation to split this up into separate pieces, adding the field and making sure the connectors pick that up and then populating it in a separate PR once that's done, or are you just calling out that we should make sure everything's picked up the recent allow-unknown-fields change before merging this? If it's the latter, I think the current state of things is:
TL;DR it looks like only ATF connectors still need any work, we would need to bump their Flow version pin and make sure they're all released before merging this.
The integration tests do actually pull down the latest flowctl ( |
Yeah I think making sure everything's picked up the allow-unknown-fields change would be the practical way to go about it. Doesn't seem like there's much additional needed for that.
I was probably wrong about this actually. I was thinking that since |
Okay, thanks. I've uploaded estuary/airbyte#360 for the ATF connectors and will make sure that's merged and pushed to all 40-something of them before this gets merged. |
Add the field to the capture Open RPC and thread it through all construction and destructure sites, which default or ignore it. It's actually populated in a later commit. The regenerated protobuf bindings follow in the next commit.
Checked-in output of the Rust and Go protobuf generators for the preceding capture Open change. No hand-written edits.
Defer endpoint config decryption until after the connector's spec response is available, capturing the sealed config from the matched endpoint. This prepares for overlay-aware decryption, which needs the connector's config schema to validate overlay property non-sensitivity.
Decrypt endpoint configuration with unseal::overlay::decrypt_with_overlay, which applies sops.overlay properties subject to config schema validation.
Thread the sealed endpoint configuration into the Open request's sealed_config_json field, so running capture connectors can emit a configUpdate which adjusts their nonsensitive sops.overlay without re-encrypting the whole configuration.
Add the field to the materialize Open RPC and thread it through all construction and destructure sites, which default or ignore it. It's actually populated in a later commit. The regenerated protobuf bindings follow in the next commit.
Checked-in output of the Rust and Go protobuf generators for the preceding materialize Open change. No hand-written edits.
Defer endpoint config decryption until after the connector's spec response is available, capturing the sealed config from the matched endpoint. This prepares for overlay-aware decryption, which needs the connector's config schema to validate overlay property non-sensitivity. Dekaf decrypts its own nested config outside this path, so its sealed config is None and nothing is decrypted here.
Decrypt endpoint configuration with unseal::overlay::decrypt_with_overlay, which applies sops.overlay properties subject to config schema validation.
Thread the sealed endpoint configuration into the Open request's sealed_config_json field, so running materialization connectors can emit a configUpdate which adjusts their nonsensitive sops.overlay without re-encrypting the whole configuration. Dekaf has no sealed config on this path, so its Open is left unpopulated.
Removes an unnecessary `display_ptr` helper function and tweaks the nonsensitivity validation logic to handle an edge case around empty objects in the overlay correctly.
Part of estuary/flow#2985, we need all connectors to have picked up the "tolerate unknown fields" change from Flow commit 184db0e before merging estuary/flow#3119 which introduces a new sealed endpoint config property to the Open RPCs. As far as I'm aware everything else either has this change or didn't need it, but airbyte-to-flow connectors still need it.
This could probably be folded into a prior commit but I'm just keeping up with codebase churn so the PR can land cleanly.
willdonnelly
force-pushed
the
wgd/2026-06-04-overlay-aware-configs
branch
from
fb74e33 to
3ed5f64
Compare
|
I updated the ATF connectors on Friday and have confirmed that the CI build updated the built image for them all, so I don't think there are any remaining obstacles to merging this now. |
## What's Changed * runtime: periodically store primary FSMHints of V2 shards by @williamhbaker in estuary/flow#3027 * mise build:flowctl for a local flowctl binary by @mdibaiee in estuary/flow#3031 * go.mod: bump gazette to latest by @williamhbaker in estuary/flow#3033 * data-plane-controller: dns and s3 passthroughs by @qaoj in estuary/flow#3022 * Self-service private-link configuration by @jshearer in estuary/flow#2944 * supabase: scoped refresh tokens so CI writes via PostgREST instead of direct psql by @skord in estuary/flow#3013 * docs: Describe SQL Server replica suport by @willdonnelly in estuary/flow#3040 * proto-gazette: regenerate to pick up SUSPEND_KEEP by @williamhbaker in estuary/flow#3041 * docs: correct custom-column-types backfill behavior for DDL changes by @jwhartley in estuary/flow#3036 * Refresh token GraphQL operations and token exchange endpoint by @GregorShear in estuary/flow#3020 * docs: shopify stream & resource config additions by @Alex-Bair in estuary/flow#3034 * validation/collection: exclude `flow://inferred-schema` from the managed-defs redact check by @jshearer in estuary/flow#3046 * docs: update Bigtable connector permissions example by @mwillman-estuary in estuary/flow#3045 * docs: disable auto-discover for multi-binding file source captures by @jwhartley in estuary/flow#3037 * docs: add streams and API pinning details to stripe-native by @nicolaslazo in estuary/flow#3055 * data-plane-controller: restart setting by @qaoj in estuary/flow#3048 * deploy: route control-plane services through the VPC NAT for a stable egress IP by @skord in estuary/flow#3044 * docs: document the Exclude Flow Document (no_flow_document) materialization option by @jwhartley in estuary/flow#3018 * oidc-discovery-server: survive direct VPC egress cold-start on deploy by @skord in estuary/flow#3062 * docs: clarify retain_existing_data_on_backfill requires allow_existing_tables_for_new_bindings by @jwhartley in estuary/flow#3049 * data-plane-controller: restart on ansiblehost by @qaoj in estuary/flow#3066 * flow-web: Bump version to release by @jshearer in estuary/flow#3068 * ops-catalog: misc updates by @williamhbaker in estuary/flow#3067 * proto-flow: tolerate unknown JSON fields in connector protocols by @williamhbaker in estuary/flow#3059 * flowctl: add raw split-shards to scale out V2 tasks by @williamhbaker in estuary/flow#3021 * supabase: qualify replace_data_plane_releases DELETE for PostgREST by @skord in estuary/flow#3076 * docs: kcat recipes for testing a Dekaf topic by @jwhartley in estuary/flow#3072 * Docs: document per-prefix alert scoping and configurable thresholds by @jwhartley in estuary/flow#3039 * notifications: trial bucket retention is 20 days, not 30 by @jwhartley in estuary/flow#3074 * runtime/container: use ops::decode::Decoder for log lines by @williamhbaker in estuary/flow#3091 * data-plane-controller: remove restart flag by @qaoj in estuary/flow#3087 * runtime-next: durably seed initial connector state as {} to match V1 by @williamhbaker in estuary/flow#3081 * runtime-next: don't abandon the final capture transaction on connector EOF by @jgraettinger in estuary/flow#3090 * Docs: Remove an IP address from GCP allowed list by @jwhartley in estuary/flow#3080 * docs: remove static data plane IP list, reference dashboard by @jwhartley in estuary/flow#3097 * dekaf: improve logging by @williamhbaker in estuary/flow#3078 * agent: make startup logging consistent by @williamhbaker in estuary/flow#3098 * shuffle: make the shuffle disk limit configurable per-task by @jgraettinger in estuary/flow#3096 * runtime-next: automatically split journals under sustained append-rate throttling by @dgreer-dev in estuary/flow#3029 * billing: add tenant billing contact fields and per-tenant controller by @jshearer in estuary/flow#2902 * flowctl-go(api test): raise test-shard readiness window from ~3s to ~30s by @jshearer in estuary/flow#3101 * runtime: strip the V2 committed-close marker for derivations by @williamhbaker in estuary/flow#3100 * deps: bump aws-lc-sys and lz4_flex for security advisories by @skord in estuary/flow#2875 * data-plane-controller: fix db timeout by @qaoj in estuary/flow#3105 * agent-api: survive direct VPC egress cold-start on instance startup by @skord in estuary/flow#3109 * docs: avoiding backfills during a database failover or host change by @jwhartley in estuary/flow#3086 * docs: clarify column-level SELECT grants unsupported for MySQL/MariaDB CDC by @jwhartley in estuary/flow#3085 * json: require RFC3339 'T' separator in date-time format validator by @jacobmarble in estuary/flow#3116 * control-plane: reserve privileged tenant names by @jwhartley in estuary/flow#3083 * agent-api: harden startup and shutdown on Cloud Run by @skord in estuary/flow#3114 * Incidental fixes: gazette Append retry, connector-init image, materialize tear-down by @jgraettinger in estuary/flow#3121 * agent: flag to create new captures as runtime-v2 by @williamhbaker in estuary/flow#3107 * runtime-next: add minimum interval for post-txn triggers by @dgreer-dev in estuary/flow#3110 * flowctl: chunk API requests by url size rather than fixed count by @mdibaiee in estuary/flow#3123 * dekaf: use projection_constraints only, no constraints by @mdibaiee in estuary/flow#3124 * flowctl: normalize --prefix trailing slash to avoid misleading PermissionDenied by @GregorShear in estuary/flow#3075 * docs: retain_existing_data_on_backfill no longer requires allow_existing_tables_for_new_bindings by @jwhartley in estuary/flow#3125 * Restart Materializations sessions before IAM token expiry by @dgreer-dev in estuary/flow#3130 * docs: add "bring your own app" instructions for source-quickbooks by @nicolaslazo in estuary/flow#3135 * flowctl: re-tool `preview` on the runtime-next stack by @jgraettinger in estuary/flow#3117 * Adding support for updating the quotas when payment info changes by @bbartman in estuary/flow#3127 * control-plane-api: add unauthenticated publicDataPlanes GraphQL query by @GregorShear in estuary/flow#3129 * go/bindings: update snapshots for materialize-sqlite ser_policy by @dgreer-dev in estuary/flow#3142 * Revert "flowctl: replace `preview` with the runtime-next implementation" by @jgraettinger in estuary/flow#3141 * local: per-checkout stacks — every checkout runs its own isolated stack by @jgraettinger in estuary/flow#3137 * dekaf: log tls handshake eof instead of returning as error by @danielnelson in estuary/flow#3143 * Updating migrations to better support testing. by @bbartman in estuary/flow#3146 * shuffle: raise causal-hint stall timeout to 15m; lower prune horizon to 2GB by @jgraettinger in estuary/flow#3148 * ci: fix Platform Build by packaging Go binaries from per-checkout $GOBIN by @jgraettinger in estuary/flow#3149 * capture+materialize: Support Nonsensitive Field Overlays by @willdonnelly in estuary/flow#3119 * Docs: The Great Description-ing by @aeluce in estuary/flow#3057 * runtime: fix recursive read-lock deadlock in capture-v2 buildJoin by @jgraettinger in estuary/flow#3156 * validation: surface write-schema redact annotations on projections of split-schema collections by @GregorShear in estuary/flow#3147 * control-plane-api: retry Stripe customer search on index-lag misses by @jgraettinger in estuary/flow#3157 * flowctl: multi-shard --fixture for raw preview-next by @mdibaiee in estuary/flow#3154 * dekaf: prevent cached `TimeoutNoData` fetch responses from incorrectly signaling EOF by @jshearer in estuary/flow#3153 * mise: remove Lima VM host share and other VM tweaks by @jgraettinger in estuary/flow#3163 * docs: document connecting Azure Private Link to native Azure resources by @jwhartley in estuary/flow#3138 * docs: explain why exclusiveCollectionFilter needs few enabled bindings by @jwhartley in estuary/flow#3126 ## New Contributors * @bbartman made their first contribution in estuary/flow#3127 **Full Changelog**: estuary/flow@v0.6.10...v0.6.11 Co-authored-by: mdibaiee <mdibaiee@users.noreply.github.com>
3 participants
Description:
This PR:
nonsensitive: trueJSON schema annotation for endpoint configs.unseal::decrypt_with_overlayhelper function which validates overlay non-sensitivity and does the extract-decrypt-merge dance around overlay properties and the main config.sealed_config_jsonfields to theOpenRPCs in the capture and materialization protocols, which will be used to provide the encrypted endpoint config to connectors. Currently they only receive the decrypted config, but in order to produceconfigUpdates which modify their own configuration via the overlays they'll need access to the raw encrypted one too.unseal::decrypt_sopsforunseal::decrypt_with_overlayin those places. This is basically the followup to the previous refactor, I just found it cleaner to make them separate commits.sealed_config_jsonproperty of Open RPCs.Since I'm not super familiar with the runtime codebase I've broken this down into fairly small, atomic commits so that I could review them individually and be pretty confident they're correct.
Part of #2985
Workflow steps:
Nothing really changes. In theory someone could manually produce a task with an endpoint overlay but nothing will be generating them automatically yet, because we need the runtime to fully support this before we start using the feature.